Skip to content

refactor(floresta-chain): apply strict error handling in flat_chain_store#1075

Open
Vedd-Patel wants to merge 1 commit into
getfloresta:masterfrom
Vedd-Patel:feat-error-handling-flat-chain-store
Open

refactor(floresta-chain): apply strict error handling in flat_chain_store#1075
Vedd-Patel wants to merge 1 commit into
getfloresta:masterfrom
Vedd-Patel:feat-error-handling-flat-chain-store

Conversation

@Vedd-Patel

@Vedd-Patel Vedd-Patel commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Description and Notes

related issue: #1053

This PR improves systematic error handling in flat_chain_store.rs, focusing on clearer error semantics and correct propagation paths

what changed

  • reworked chainstore error modeling to use ChainstoreError with clearer variants and messages
  • preserved error sources for internal failures via Internal(Box<dyn Error + Send + Sync>)
  • improved index/probing error behavior so not-found/full-index paths are handled correctly
  • added explicit propagation for InvalidValidationIndex in save_roots_for_block
  • kept module-level strict clippy guards for error-handling pattern

Contributor Checklist

  • I've followed the contribution guidelines
  • I've verified one of the following:
    • Ran just pcc (recommended but slower)
    • Ran just lint-features '-- -D warnings' && cargo test --release
    • Confirmed CI passed on my fork
  • I've linked any related issue(s) in the sections above

@moisesPompilio moisesPompilio added the reliability Related to runtime reliability, stability and production readiness label May 19, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in Floresta May 19, 2026
@moisesPompilio moisesPompilio added this to the SoB/2026 milestone May 19, 2026
@moisesPompilio moisesPompilio requested a review from jaoleal May 19, 2026 12:19
@moisesPompilio moisesPompilio moved this from Backlog to Needs review in Floresta May 19, 2026
@Vedd-Patel

Vedd-Patel commented May 19, 2026

Copy link
Copy Markdown
Author

@moisesPompilio you can also take a look!
I would love your feedbacks if any

@jaoleal jaoleal moved this from Needs review to In progress in Floresta May 19, 2026
Davidson-Souza
Davidson-Souza reviewed May 19, 2026
View reviewed changes

@Davidson-Souza Davidson-Souza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few architectural questions.

It also looks like there was a regression with how we handle dirs.

Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
jaoleal
jaoleal reviewed May 19, 2026
View reviewed changes

@jaoleal jaoleal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be interesting if you could split the changes, itll be easier to review

Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
@Vedd-Patel

Vedd-Patel commented May 19, 2026

Copy link
Copy Markdown
Author

It would be interesting if you could split the changes, itll be easier to review

from next time i'll keep this in mind

@jaoleal

jaoleal commented May 19, 2026

Copy link
Copy Markdown
Member

Guys, the only recoverable error inside the flatchainstore is HeadersNotFound... Should we review the plan ?

@Davidson-Souza

@Davidson-Souza

Davidson-Souza commented May 19, 2026

Copy link
Copy Markdown
Member

Guys, the only recoverable error inside the flatchainstore is HeadersNotFound

That's not true. OversizedAccumulator, CorruptedDatabase and InvalidValidationIndex are fixed by a reindex.

@Vedd-Patel Vedd-Patel force-pushed the feat-error-handling-flat-chain-store branch from 8c4069e to 4668b46 Compare May 29, 2026 05:29
lorenzolfm
lorenzolfm reviewed May 31, 2026
View reviewed changes

@lorenzolfm lorenzolfm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Vedd-Patel. Thanks for your work on this. Your PR is going on the right direction :)

I've made some comments about simplifying the Internal variant, the value of some tests you added and notes about some changes that you may want to avoid to keep this PR on track with your goal and keep it easy for contributors to review.

Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs
@Vedd-Patel Vedd-Patel force-pushed the feat-error-handling-flat-chain-store branch from 4668b46 to c22550a Compare June 5, 2026 12:33
@Vedd-Patel Vedd-Patel requested a review from lorenzolfm June 5, 2026 12:35
@lorenzolfm

lorenzolfm commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Addressed some comments. By the way there are still some renames on variable names that I think you should avoid doing. The smaller the diff, the higher the quality of reviews you'll get :)

@Vedd-Patel Vedd-Patel force-pushed the feat-error-handling-flat-chain-store branch from c22550a to fb91fef Compare June 9, 2026 07:34
@Vedd-Patel

Vedd-Patel commented Jun 9, 2026

Copy link
Copy Markdown
Author

@lorenzolfm i've checked and reverted almost all of the renames, the only rename i'd like to keep is FlatChainStoreError -> ChainstoreError if you think this rename could cause any issues or if you'd prefer to keep the original name, then let me know and i'll revert that one as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaoleal jaoleal Awaiting requested review from jaoleal

@Davidson-Souza Davidson-Souza Awaiting requested review from Davidson-Souza

@lorenzolfm lorenzolfm Awaiting requested review from lorenzolfm

Assignees

@Vedd-Patel Vedd-Patel

Labels

reliability Related to runtime reliability, stability and production readiness

Projects

Floresta
Status: In progress

Milestone

SoB/2026

Development

Successfully merging this pull request may close these issues.

5 participants

@Vedd-Patel @jaoleal @Davidson-Souza @lorenzolfm @moisesPompilio